-
-
Notifications
You must be signed in to change notification settings - Fork 226
Consolidate volume stream control into volume up/down actions #1854
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Updated ActionData.Volume.Up and Down to include volumeStream parameter - Created VolumeActionBottomSheet.kt with Compose UI for stream selection - Updated CreateActionDelegate to handle new volume action configuration - Updated ActionUiHelper to display stream in action descriptions - Updated PerformActionsUseCase to use stream parameter when executing actions - Added string resources for volume configuration UI Co-authored-by: jambl3r <[email protected]>
- Updated ActionDataEntityMapper to read/write volumeStream from entity extras - Ensures backward compatibility for existing keymaps without stream data Co-authored-by: jambl3r <[email protected]>
- Use safe call operator instead of non-null assertion when reading stream type - Returns null for invalid stream types instead of crashing - Update CHANGELOG with new feature description Co-authored-by: jambl3r <[email protected]>
- Replace non-null assertions with safe call operators in entity extras writing - Gracefully handles potential missing map entries Co-authored-by: jambl3r <[email protected]>
sds100
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot implement my comments
CHANGELOG.md
Outdated
| - #727 Actions to send SMS messages: "Send SMS" and "Compose SMS" | ||
| - #1819 Explain how to enable the accessibility service restricted setting | ||
| - #661 Action to execute shell commands. | ||
| - Volume up/down actions now allow selecting a specific audio stream (e.g., music, notifications) to control, consolidating the functionality of increase/decrease stream actions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot Reword this to something like "consolidated volume and stream actions"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to "Consolidated volume and stream actions." in commit 3a6ac16
|
|
||
| var httpRequestBottomSheetState: ActionData.HttpRequest? by mutableStateOf(null) | ||
| var smsActionBottomSheetState: SmsActionBottomSheetState? by mutableStateOf(null) | ||
| var volumeUpActionState: VolumeActionBottomSheetState? by mutableStateOf(null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just use the same volumeActionState and store which ActionId is being used. Then use that to figure out which action to create when done is clicked. Also show different bottom sheet title depending on the action.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consolidated into single volumeActionState with actionId field to determine which action to create. The bottom sheet title now dynamically changes based on the actionId. See commit 3a6ac16
|
|
||
| @Serializable | ||
| data class Up(val showVolumeUi: Boolean) : Volume() { | ||
| data class Up( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you delete the streams in ActionData. Mark the ActionIds as deprecated. The ActionDataEntityMapper should be backwards compatible with the old Stream ActionIds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deprecated VOLUME_INCREASE_STREAM and VOLUME_DECREASE_STREAM ActionIds, and marked Volume.Stream classes as deprecated. Kept the classes for full backward compatibility - old stream ActionIds will still deserialize correctly via ActionDataEntityMapper. See commit 3a6ac16
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot delete ActionData.Stream classes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deleted ActionData.Volume.Stream classes completely. Old stream actions are now automatically migrated to Volume.Up/Down with stream parameter via ActionDataEntityMapper. See commit 39ca2c4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deleted ActionData.Volume.Stream classes completely. Old stream actions are now automatically migrated to Volume.Up/Down with stream parameter via ActionDataEntityMapper. See commit 39ca2c4
- Consolidated volumeUpActionState and volumeDownActionState into single volumeActionState - Added actionId to VolumeActionBottomSheetState to determine which action to create - Updated bottom sheet to use actionId for dynamic title selection - Merged onDoneConfigVolumeUpClick and onDoneConfigVolumeDownClick into single handler - Deprecated VOLUME_INCREASE_STREAM and VOLUME_DECREASE_STREAM ActionIds - Deprecated Volume.Stream classes (kept for backward compatibility) - Updated CHANGELOG wording to "Consolidated volume and stream actions" Co-authored-by: sds100 <[email protected]>
sds100
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot do the comments
|
|
||
| @Serializable | ||
| data class Up(val showVolumeUi: Boolean) : Volume() { | ||
| data class Up( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot delete ActionData.Stream classes.
- Removed deprecated Stream sealed class and Increase/Decrease subclasses - Updated ActionDataEntityMapper to convert old VOLUME_INCREASE_STREAM and VOLUME_DECREASE_STREAM to Volume.Up and Volume.Down with stream parameter - Updated CreateActionDelegate to redirect old stream action IDs to new bottom sheet - Removed all Stream references from PerformActionsUseCase, ActionUiHelper, ConfigActionsUseCase, ActionUtils, and KeyMapActionsComparator - Maintained full backward compatibility - old keymaps with stream actions will be automatically migrated to new format Co-authored-by: sds100 <[email protected]>
Volume up/down actions now support selecting specific audio streams (music, notifications, etc.), eliminating the need for separate increase/decrease stream actions.
Changes
Data Model
ActionData.Volume.UpandActionData.Volume.Downwith nullablevolumeStream: VolumeStream?parameterVOLUME_INCREASE_STREAMandVOLUME_DECREASE_STREAMActionIdsVolume.Streamclasses completely - old stream actions are automatically migrated to new formatUI (Jetpack Compose)
VolumeActionBottomSheetwith radio button stream selection and "show volume dialog" checkboxvolumeActionStatewithactionIdfieldChooseActionScreenalongside existing action bottom sheetsAction Execution & Serialization
PerformActionsUseCaseto pass stream parameter to audio adapterActionDataEntityMapperserializes stream using existingEXTRA_STREAM_TYPEconstantVOLUME_INCREASE_STREAMactions converted toVolume.Upwith stream parameter,VOLUME_DECREASE_STREAMconverted toVolume.Downwith stream parameterDisplay
ActionUiHelpershows stream in description when selected (e.g., "Increase Music stream")Example
Existing keymaps with old stream actions are automatically migrated to the new
Volume.Up/Volume.Downformat when loaded, ensuring seamless transition without data loss.Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.